Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sample stream and release metadata, and release index #207

Merged
merged 6 commits into from
Jul 12, 2019
Merged

Add sample stream and release metadata, and release index #207

merged 6 commits into from
Jul 12, 2019

Conversation

bgilbert
Copy link
Contributor

From #98.

Copy link
Contributor

@sinnykumari sinnykumari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments, otherwise LGTM

@@ -0,0 +1,17 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we include stream name in release-index.json as well? Can be useful to figure out stream while looking at the json file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The release-index is inside of a stream specific location (e.x.: s3://fcos-builds/prod/streams/<stream>/release-index.json)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followup: the release-index is also specific to an architecture. Thus, architecture label should be part of the location, or somewhere in the index.

Copy link
Contributor

@arithx arithx Jun 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucab: Reading the code again with more coffee leads me to believe that I misread something earlier and that is not the case currently.

Each entry in release-index points to an endpoint for the release.json which IS stream specific but is NOT architecture specific.

e.x.:

s3://fcos-builds/prod/streams/foo/release-index.json is the release-index for the foo stream. An example entry would be something like:

{
    "commit": "<hash>",
    "version": "30.123.45",
    "endpoint": "https://fcos-builds.s3.amazonaws.com/prod/streams/foo/builds/30.123.45/release.json"
}

{
"releases": [
{
"commit": "<hash>",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From comment #98 (comment), I believe commit and version we are referring here is ostree commit and version. One thing to note that, we will have same ostree version number for a release across all arches but commit hash will be different for different arches.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of these fields are unique, we can have different commits with the same version and it won't conflict.

@bgilbert
Copy link
Contributor Author

Renamed release index sample to reduce confusion. Will address other comments next week.

# slug is the best we can do.
image: fedora_coreos_stable

updates:
Copy link
Contributor

@lucab lucab Jun 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure about the indentation of this. I don't think we want to have different update metadata for each architecture, to avoid exploding RelEng complexity. If so, I think this can become a top-level section on its own.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, in hindsight that choice seems somewhat arbitrary. If we want different update targets for different arches, presumably we'd also want them for different platforms. I don't want to introduce that level of complexity right now, so I'll switch this to a single value.

bgilbert added 4 commits July 8, 2019 15:54
Might as well keep the document self-contained.
It seems more important to distinguish per-platform targets than
per-arch ones, and that would introduce a lot of complexity.  So, for
now, keep a single update target for the entire stream.
@bgilbert
Copy link
Contributor Author

bgilbert commented Jul 8, 2019

Updated.

Copy link
Contributor

@sinnykumari sinnykumari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

sinnykumari added a commit to coreos/fedora-coreos-stream-generator that referenced this pull request Jul 10, 2019
sinnykumari added a commit to coreos/fedora-coreos-stream-generator that referenced this pull request Jul 10, 2019
@bgilbert bgilbert merged commit e54742e into coreos:master Jul 12, 2019
@bgilbert bgilbert deleted the metadata branch July 12, 2019 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants